Skip to content

Update chroma to v2.20.0 #35220

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Aug 11, 2025
Merged

Conversation

sebastianertz
Copy link
Contributor

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Aug 6, 2025
@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Aug 6, 2025
@silverwind
Copy link
Member

Unit test failure is related:

 --- FAIL: TestRender_Source (0.01s)
    orgmode_test.go:97: 
        	Error Trace:	/home/runner/work/gitea/gitea/modules/markup/orgmode/orgmode_test.go:97
        	            				/home/runner/work/gitea/gitea/modules/markup/orgmode/orgmode_test.go:100
        	Error:      	Not equal: 
        	            	expected: "<div class=\"src src-go\">\n<pre><code class=\"chroma language-go\"><span class=\"c1\">// HelloWorld prints &#34;Hello World&#34;</span>\n<span class=\"kd\">func</span> <span class=\"nf\">HelloWorld</span><span class=\"p\">()</span> <span class=\"p\">{</span>\n\t<span class=\"nx\">fmt</span><span class=\"p\">.</span><span class=\"nf\">Println</span><span class=\"p\">(</span><span class=\"s\">&#34;Hello World&#34;</span><span class=\"p\">)</span>\n<span class=\"p\">}</span></code></pre>\n</div>"
        	            	actual  : "<div class=\"src src-go\">\n<pre><code class=\"chroma language-go\"><span class=\"c1\">// HelloWorld prints &#34;Hello World&#34;</span><span class=\"w\">\n</span><span class=\"w\"></span><span class=\"kd\">func</span><span class=\"w\"> </span><span class=\"nf\">HelloWorld</span><span class=\"p\">()</span><span class=\"w\"> </span><span class=\"p\">{</span><span class=\"w\">\n</span><span class=\"w\">\t</span><span class=\"nx\">fmt</span><span class=\"p\">.</span><span class=\"nf\">Println</span><span class=\"p\">(</span><span class=\"s\">&#34;Hello World&#34;</span><span class=\"p\">)</span><span class=\"w\">\n</span><span class=\"w\"></span><span class=\"p\">}</span></code></pre>\n</div>"

@sebastianertz
Copy link
Contributor Author

Does the unit test need to be adjusted?
I'm not familiar with the Go language. But I would be happy if Chroma received an update. Because of the new languages in Chroma.

@silverwind
Copy link
Member

Yes, apparently chrome tokenization changed in this version which causes the failure. We need to update the test and verify syntax highlighting works as expected.

@a1012112796
Copy link
Member

Unit test failure is related:

 --- FAIL: TestRender_Source (0.01s)
    orgmode_test.go:97: 
        	Error Trace:	/home/runner/work/gitea/gitea/modules/markup/orgmode/orgmode_test.go:97
        	            				/home/runner/work/gitea/gitea/modules/markup/orgmode/orgmode_test.go:100
        	Error:      	Not equal: 
        	            	expected: "<div class=\"src src-go\">\n<pre><code class=\"chroma language-go\"><span class=\"c1\">// HelloWorld prints &#34;Hello World&#34;</span>\n<span class=\"kd\">func</span> <span class=\"nf\">HelloWorld</span><span class=\"p\">()</span> <span class=\"p\">{</span>\n\t<span class=\"nx\">fmt</span><span class=\"p\">.</span><span class=\"nf\">Println</span><span class=\"p\">(</span><span class=\"s\">&#34;Hello World&#34;</span><span class=\"p\">)</span>\n<span class=\"p\">}</span></code></pre>\n</div>"
        	            	actual  : "<div class=\"src src-go\">\n<pre><code class=\"chroma language-go\"><span class=\"c1\">// HelloWorld prints &#34;Hello World&#34;</span><span class=\"w\">\n</span><span class=\"w\"></span><span class=\"kd\">func</span><span class=\"w\"> </span><span class=\"nf\">HelloWorld</span><span class=\"p\">()</span><span class=\"w\"> </span><span class=\"p\">{</span><span class=\"w\">\n</span><span class=\"w\">\t</span><span class=\"nx\">fmt</span><span class=\"p\">.</span><span class=\"nf\">Println</span><span class=\"p\">(</span><span class=\"s\">&#34;Hello World&#34;</span><span class=\"p\">)</span><span class=\"w\">\n</span><span class=\"w\"></span><span class=\"p\">}</span></code></pre>\n</div>"

looks that's because in alecthomas/chroma@d0ad679 , golang rulers was updated. the ui is ok in my test. so I think just update the test code is enough. @silverwind @sebastianertz

image

@silverwind
Copy link
Member

Yeah just update the failing test value assertions.

BTW I wish we would have snapshot-based testing (for example https://github.com/gkampitakis/go-snaps), then we could automate such updates with a single command (like make test-backend-update).

@wxiaoguang
Copy link
Contributor

BTW I wish we would have snapshot-based testing (for example https://github.com/gkampitakis/go-snaps), then we could automate such updates with a single command (like make test-backend-update).

It only introduces more garbage into code base. Every test should be designed & written carefully, and has a clear purpose, otherwise, there is already a lot of incomprehensible & unmaintainable code like

func TestTotal_RenderString(t *testing.T) {


For this one, it could simply test some simple code like this to confirm that "the render works".

#+begin_src c
int a;
#+end_src

@github-actions github-actions bot added the modifies/go Pull requests that update Go code label Aug 11, 2025
Signed-off-by: wxiaoguang <[email protected]>
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Aug 11, 2025
@silverwind
Copy link
Member

silverwind commented Aug 11, 2025

Yeah that's ok as well and less likely to break between updates. I still think snapshot testing is valuable, so I would still use it if a need arises.

@silverwind silverwind merged commit 9b5a3e9 into go-gitea:main Aug 11, 2025
26 checks passed
@GiteaBot GiteaBot added this to the 1.25.0 milestone Aug 11, 2025
@sebastianertz sebastianertz deleted the update-chroma branch August 11, 2025 12:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. modifies/dependencies modifies/go Pull requests that update Go code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants